-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(router): fix router ports #2757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryThis PR attempts to fix router edge deletion and improve router prompting, but introduces a critical bug in route ID generation that will break workflow execution. What ChangedThe PR makes 4 main changes to the router v2 block:
Critical Issue FoundRoute ID generation inconsistency in condition-input.tsx (line 627-632):
This breaks the entire routing system because:
Additional ConcernThe silent fallback in router-handler.ts masks routing failures. While it prevents crashes, users won't know when the router isn't working properly, making debugging difficult. RecommendationDO NOT MERGE until the route ID generation bug is fixed. The edge deletion and autoconnect fixes are good, but the ID generation issue will cause runtime failures. Confidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant ConditionInput
participant WorkflowBlock
participant WorkflowCanvas
participant RouterHandler
participant EdgeManager
Note over User,EdgeManager: Router Port Creation & Connection Flow
User->>ConditionInput: Add new route
ConditionInput->>ConditionInput: Generate route ID (BUG: uses timestamp)
ConditionInput->>ConditionInput: Save route with ID "blockId-route-123456789"
WorkflowBlock->>WorkflowBlock: Parse routes from JSON
WorkflowBlock->>WorkflowBlock: Create handle with id="router-blockId-route-123456789"
User->>WorkflowCanvas: Drag connection from router port
WorkflowCanvas->>WorkflowCanvas: Auto-detect router handle
WorkflowCanvas->>WorkflowCanvas: Create edge with sourceHandle="router-blockId-route-123456789"
Note over User,EdgeManager: Workflow Execution Flow
User->>RouterHandler: Execute workflow
RouterHandler->>RouterHandler: Call LLM with context + route descriptions
RouterHandler->>RouterHandler: LLM returns route ID
alt Valid route ID
RouterHandler->>RouterHandler: Find route in parsed routes
RouterHandler->>EdgeManager: Look for edge with sourceHandle="router-{routeId}"
EdgeManager->>EdgeManager: Match handle prefix "router-"
EdgeManager->>RouterHandler: Return target block
RouterHandler->>RouterHandler: Execute target block
else Invalid route ID (NEW)
RouterHandler->>RouterHandler: Log warning
RouterHandler->>RouterHandler: Fallback to routes[0]
Note right of RouterHandler: Silent failure - masks issues
RouterHandler->>EdgeManager: Use first route ID
end
Note over User,EdgeManager: Edge Deletion Flow (FIXED)
User->>ConditionInput: Delete route
ConditionInput->>ConditionInput: Get route ID
ConditionInput->>ConditionInput: Build handlePrefix "router-{routeId}"
ConditionInput->>WorkflowCanvas: Remove edges matching prefix
WorkflowCanvas->>WorkflowCanvas: Delete connected edges
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 2 comments
Additional Comments (1)
When workflow-block.tsx parses existing routes, it expects IDs in the format This inconsistency means:
Fix: Generate sequential route numbers instead of timestamps: Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/condition-input/condition-input.tsx
Line: 627:632
Comment:
The route ID generation is inconsistent and will cause edge matching issues. The default route uses `route1` (line 178: `generateStableId(blockId, 'route1')`), but new routes use `route-${Date.now()}` which creates a different format.
When workflow-block.tsx parses existing routes, it expects IDs in the format `${blockId}-route${index + 1}` (line 867), which would be `blockId-route1`, `blockId-route2`, etc.
This inconsistency means:
1. New routes created by the user will have IDs like `blockId-route-1234567890`, not `blockId-route2`
2. Handles created for these routes won't match the expected pattern
3. Edge connections and execution flow will break
**Fix**: Generate sequential route numbers instead of timestamps:
```suggestion
// Count existing routes to determine next route number
const routeCount = conditionalBlocks.filter(b => b.title.startsWith('route')).length
const newBlockId = isRouterMode
? generateStableId(blockId, `route${routeCount + 1}`)
: generateStableId(blockId, `else-if-${Date.now()}`)
const newBlock: ConditionalBlock = {
id: newBlockId,
title: isRouterMode ? `route${routeCount + 1}` : '',
```
How can I resolve this? If you propose a fix, please make it concise. |
Summary
Fix bugs with router (edge deletion, prompting)
Type of Change
Testing
Manual
Checklist